docs(readme): behavior accuracy, dry-run note, config vs registry locking#5
Conversation
Replace the previous default mainline-only remote fetch with git fetch --all --prune per repository (honoring sync_tags). Introduce --mainline-fetch to opt into the old MainlineFetchRemotes behavior. Remove the redundant --fetch-only flag and update docs, UAT, and tests. Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
README and CLAUDE describe default full fetch, --fetch-mainline, --sync with branch_mode (including all-branches), and --risky on sync. Rename CLI flag from --mainline-fetch to --fetch-mainline and fix variable naming; update UAT labels and test messages. Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
Default fetch is git fetch --all without --prune. Add global fetch_prune, registry fetch_prune, local rain.fetchprune, and --prune/--no-prune flags (CLI wins). Wire RainOptions.FetchPrune through fetchOnly, MainlineFetchRemotes, and RainRepository. Document precedence and pruning risks in README with a warning callout. Add unit tests for resolution order and ReadRainFetchPrune. Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
Dry-run now prints resolved Fetch --prune for --sync as well as fetch paths. Registry Upsert for-loop indentation matches other methods after gofmt.
…guard - applyRepoFetchPrune: skip git config read when CLI sets prune; warn and fall back on ReadRainFetchPrune errors - Reject --fetch-mainline with full-sync triggers; add test - runDryRun: show fetch --prune resolution for --sync path too - Fix Upsert for-loop indentation (gofmt) - UAT scenario 7 echo matches prune-opt-in default - CLAUDE/README: prune precedence and --fetch-mainline note - Replace misleading frozen label (fetch --all) Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
- Resolve conflicts in cmd/root.go and cmd/root_test.go (keep fetch-mainline note and dry-run prune test). - SaveConfig and WriteExampleConfig: exclusive flock on config.toml.lock, PID-scoped temp file, atomic rename (avoids corruption / stale .tmp). - Add github.com/gofrs/flock; concurrent SaveConfig test. Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
Match full-branch-hydration triggers to computeFullSync, fix default_mode example, document prune precedence and backup ref shape. Add a dry-run pun and a durability note contrasting repos.toml locking with config.toml atomic writes and remaining race scenarios.
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughDocs and TUI behavior clarified and renamed (push→sync); config defaults/validation changed to Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (TUI)"
participant TUI as "TUI (picker)"
participant Scanner as "Scanner (filesystem/git)"
participant Channel as "tuiRepoChan (buffered)"
participant Bridge as "bridge goroutine / drain"
participant CancelCtx as "Scan cancel context"
Note over TUI,Scanner: Streaming scan start
User->>TUI: open picker
TUI->>Scanner: Start ScanRepositoriesStream(ctx)
Scanner-->>Channel: send repo items (async)
Channel-->>TUI: deliver items (render)
alt User selects repo or exits
TUI->>CancelCtx: cancelScan() (immediately)
CancelCtx-->>Scanner: context cancelled
Scanner--xChannel: stop sending / close channel
TUI->>Bridge: wait for bridgeDone while draining Channel
TUI->>Channel: drain remaining items
Bridge-->>TUI: bridgeDone
TUI->>TUI: shutdown UI and return
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
- Example config and TUI config screen use sync-default/sync-all/ sync-current-branch/leave-untouched (matches ParseMode). - Validate global.default_mode strictly; empty normalizes to sync-default. - Trim git-fire mirroring from CLI flag text and README; tighten CLAUDE.md.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
244-244:⚠️ Potential issue | 🟡 MinorResolve the
--branch-modetrigger contradiction.Line 244 says
--branch-modeapplies “With--sync”, but Line 28 and Line 305 say any CLI--branch-modetriggers full branch hydration even without--sync. These statements conflict and will confuse users.Suggested doc fix
-| `--branch-mode` | With `--sync`: `mainline`, `checked-out`, `all-local`, or `all-branches` (overrides config for this run) | +| `--branch-mode` | Branch selection mode: `mainline`, `checked-out`, `all-local`, or `all-branches`. On CLI, providing this flag enters the full branch-hydration path for the run (overrides config). |Based on learnings: Use
RainRepositoryfor local branch hydration when--sync, non-mainlinebranch_mode, or risky-only config forces full sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 244, Update the README to remove the contradictory wording that `--branch-mode` only applies "With `--sync`" and clearly document the actual behavior: state that supplying the CLI flag `--branch-mode` triggers full branch hydration even when `--sync` is not provided, and additionally explain that when `--sync` is used the system uses `RainRepository` for local branch hydration and that non-mainline `branch_mode` or a risky-only config also forces full sync; reference the flags `--branch-mode` and `--sync`, the config key `branch_mode`, and the class `RainRepository` so readers can map the behavior to the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 240: Update the README entry for `--dry-run` to clarify its behavior when
combined with `--no-scan`: mention that `--dry-run` normally skips git
fetch/branch updates but still scans the disk to list repos, and explicitly add
a caveat that if `--no-scan` is also provided, the disk scan is skipped and
`--dry-run` will not list repos from disk; reference the `--dry-run` and
`--no-scan` flags so readers understand the interaction.
---
Outside diff comments:
In `@README.md`:
- Line 244: Update the README to remove the contradictory wording that
`--branch-mode` only applies "With `--sync`" and clearly document the actual
behavior: state that supplying the CLI flag `--branch-mode` triggers full branch
hydration even when `--sync` is not provided, and additionally explain that when
`--sync` is used the system uses `RainRepository` for local branch hydration and
that non-mainline `branch_mode` or a risky-only config also forces full sync;
reference the flags `--branch-mode` and `--sync`, the config key `branch_mode`,
and the class `RainRepository` so readers can map the behavior to the
implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
…into docs/readme-dry-run-durability-locks
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/config/loader.go (1)
118-125:⚠️ Potential issue | 🟡 MinorCleanup stale tmp on
os.WriteFilefailure too.If
os.WriteFilefails mid-write (e.g. ENOSPC, permissions), a partialpath.<pid>.tmpcan be left behind — cleanup only happens onRenamefailure. Mirror the sameos.Remove(tmp)on the WriteFile error path for symmetry and to avoid littered temp files across runs.Proposed fix
tmp := fmt.Sprintf("%s.%d.tmp", path, os.Getpid()) if err := os.WriteFile(tmp, data, 0o600); err != nil { + _ = os.Remove(tmp) return fmt.Errorf("writing temp config: %w", err) }Apply the same to
WriteExampleConfigat line 228.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/loader.go` around lines 118 - 125, The temp file created as tmp := fmt.Sprintf("%s.%d.tmp", path, os.Getpid()) is not removed if os.WriteFile(tmp, data, 0o600) fails, leaving stale tmp files; update the error path in the block that writes and renames the temp file to call os.Remove(tmp) before returning when os.WriteFile fails, and apply the same cleanup change to the analogous WriteExampleConfig function so both write-failure and rename-failure paths remove the temporary file (reference tmp, path, os.WriteFile, os.Rename, and WriteExampleConfig to locate the spots to change).
🧹 Nitpick comments (1)
internal/config/loader.go (1)
215-235: Duplication withSaveConfig— consider extracting.
WriteExampleConfigis now structurally identical toSaveConfig(lock → write tmp → rename), differing only in payload source and error-message wording. Worth extracting a smallatomicWrite(path string, data []byte) errorhelper to keep the lock/tmp/rename invariants in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/loader.go` around lines 215 - 235, WriteExampleConfig duplicates SaveConfig's lock/tmp/rename logic; extract that shared behavior into a new helper atomicWrite(path string, data []byte) error that acquires the same file lock (flock.New(path+".lock")), writes to a temp file (fmt.Sprintf("%s.%d.tmp", path, os.Getpid())), renames it into place, and cleans up on error; then update SaveConfig and WriteExampleConfig to call atomicWrite with their respective payloads (e.g., ExampleConfigTOML() output) and preserve existing error wrapping and permissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/config/loader.go`:
- Around line 108-112: Replace the blocking flock.Lock() call in SaveConfig with
a non-blocking TryLockContext: create a context with a short timeout (e.g.,
500ms) and call lock.TryLockContext(ctx, retryDelay) instead of lock.Lock(); on
timeout or error propagate the error through the existing configSaveErr
mechanism so the TUI can surface the failure; ensure you still defer unlocking
(lock.Unlock()) when TryLockContext succeeds and keep using flock.New(path +
".lock") as before.
---
Outside diff comments:
In `@internal/config/loader.go`:
- Around line 118-125: The temp file created as tmp := fmt.Sprintf("%s.%d.tmp",
path, os.Getpid()) is not removed if os.WriteFile(tmp, data, 0o600) fails,
leaving stale tmp files; update the error path in the block that writes and
renames the temp file to call os.Remove(tmp) before returning when os.WriteFile
fails, and apply the same cleanup change to the analogous WriteExampleConfig
function so both write-failure and rename-failure paths remove the temporary
file (reference tmp, path, os.WriteFile, os.Rename, and WriteExampleConfig to
locate the spots to change).
---
Nitpick comments:
In `@internal/config/loader.go`:
- Around line 215-235: WriteExampleConfig duplicates SaveConfig's
lock/tmp/rename logic; extract that shared behavior into a new helper
atomicWrite(path string, data []byte) error that acquires the same file lock
(flock.New(path+".lock")), writes to a temp file (fmt.Sprintf("%s.%d.tmp", path,
os.Getpid())), renames it into place, and cleans up on error; then update
SaveConfig and WriteExampleConfig to call atomicWrite with their respective
payloads (e.g., ExampleConfigTOML() output) and preserve existing error wrapping
and permissions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 95376c1a-20e6-4f86-b9f1-7add88bf9878
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
CLAUDE.mdREADME.mdcmd/root.gocmd/root_test.gogo.modinternal/config/config_test.gointernal/config/defaults.gointernal/config/loader.gointernal/registry/types.gointernal/ui/config_view.go
✅ Files skipped from review due to trivial changes (6)
- go.mod
- internal/registry/types.go
- CLAUDE.md
- cmd/root.go
- internal/config/defaults.go
- internal/config/config_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
Add PanelBlockWidth/PanelTextWidth/RainDisplayWidth so list, settings, rain, and PathWidthFor share one layout budget (outer margin 6 + box padding 4). Clamp list/settings rows with lipgloss; align lipgloss padding in tests. Treat tea.InterruptMsg like quit; map tea.ErrInterrupted to ErrCancelled. Co-authored-by: bschellenberger2600 <bschellenberger2600@gmail.com>
…lysis Cancel scan context immediately after the TUI returns so filepath walk and git subprocesses unwind. Buffer tuiRepoChan to reduce bridge backpressure. Use exec.CommandContext in ScanRepositoriesStream analysis; add test that a pre-cancelled scan still closes the output channel. Co-authored-by: bschellenberger2600 <bschellenberger2600@gmail.com>
Document q/ctrl+c and scan cancellation in README; point maintainers at panel_layout and runRainTUIStream shutdown in CLAUDE.md. Co-authored-by: bschellenberger2600 <bschellenberger2600@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cmd/root.go (1)
552-564:⚠️ Potential issue | 🟡 MinorBridge goroutine's
saveRegistryisn't awaited.
<-scanDoneonly waits for the scanner goroutine, not for the bridge goroutine at lines 526–536 that doessaveRegistry(reg, regPath)afterscanChancloses. On theui.ErrCancelledpath a few lines below,os.Exit(0)fires as soon as the drain returns, which can happen before the bridge finalizes and persists registry changes (e.g., newly-discovered repos,LastSeenbumps).♻️ Suggested fix
var scanErr error scanDone := make(chan struct{}) go func() { defer close(scanDone) scanErr = git.ScanRepositoriesStream(opts, scanChan) }() now := time.Now() defaultMode := git.ParseMode(cfg.Global.DefaultMode) + bridgeDone := make(chan struct{}) go func() { + defer close(bridgeDone) defer close(tuiRepoChan) for repo := range scanChan { repo, include := upsertRepoIntoRegistry(reg, repo, now, defaultMode) if include { repo.Selected = true tuiRepoChan <- repo } } saveRegistry(reg, regPath) }() ... <-scanDone + <-bridgeDone🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/root.go` around lines 552 - 564, The bridge goroutine that reads scanChan and calls saveRegistry(reg, regPath) is not awaited: current code only waits on <-scanDone (scanner) so the program can exit before the bridge finishes persisting registry changes. Fix by adding an explicit synchronization for the bridge (e.g., a done channel or sync.WaitGroup) that the bridge signals after saveRegistry returns, then wait for that signal before returning/exiting (on the ui.ErrCancelled path and the normal shutdown path); reference the bridge goroutine that closes over scanChan and calls saveRegistry(reg, regPath), the existing scanDone waiter, and the cancelScan() call to ensure the new wait happens after cancelScan() and draining.internal/ui/repo_selector.go (1)
1226-1231:⚠️ Potential issue | 🔴 CriticalRemove unused
min— CI lint is failing.golangci-lint reports
func min is unused. After switching toresolveRainBackgroundWidth/RainDisplayWidth, nothing calls the local helper anymore, and Go 1.21+ provides a built-inminanyway.🧹 Proposed fix
-func min(a, b int) int { - if a < b { - return a - } - return b -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/ui/repo_selector.go` around lines 1226 - 1231, Remove the unused local helper function min in internal/ui/repo_selector.go: delete the min(a, b int) int definition since golangci-lint flags it as unused and Go 1.21+ provides a built-in min; also search for any residual callers (e.g., inside resolveRainBackgroundWidth or RainDisplayWidth) and replace them with the language built-in min if any remain, ensuring no references to the min function are left behind.
🧹 Nitpick comments (2)
internal/ui/path_display.go (1)
84-89: Document whatreserve = 34accounts for.The measured
prefixW/suffixWexclude the dirty indicator (💧, ~3 cells) and the scroll hint (<< SCROLL PATH >>, ~19 cells) which only appears on the focused row. A single opaque34makes this easy to drift if either string is tweaked elsewhere. Consider either breaking it out or at least commenting the derivation.♻️ Example
- suffixW := lipgloss.Width(fmt.Sprintf("›) [%s] %s", repo.Mode.String(), remotesInfo)) - const reserve = 34 + suffixW := lipgloss.Width(fmt.Sprintf("›) [%s] %s", repo.Mode.String(), remotesInfo)) + // reserve = dirty indicator (" 💧" ≈ 3) + scroll hint (" << SCROLL PATH >>" ≈ 19) + safety margin. + const reserve = 34🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/ui/path_display.go` around lines 84 - 89, The hard-coded reserve = 34 is undocumented and hides the width contributions from the per-row dirty indicator and the focused-row scroll hint; update the pw calculation in path_display.go to replace the magic number with a named constant (e.g., scrollHintWidth or reservedWidth) or add a clear inline comment explaining the derivation (dirty indicator ~3 cells + scroll hint ~19 cells + any padding = 34) and use that constant when computing pw alongside prefixW and suffixW so future changes to the dirty indicator or scroll hint are obvious; reference the existing variables prefixW, suffixW, reserve (or new constant) and pw to locate where to apply the change.internal/git/scanner.go (1)
163-179: Remove theerrors.Is(err, context.Canceled)checks—they never match for killed subprocesses.When
exec.CommandContextcancels a child process,cmd.Output()returns an*exec.ExitErrorwith signal information, not an error satisfyingerrors.Is(err, context.Canceled). The subsequentctx.Err()guards already detect cancellation correctly, making these branches unreachable dead code.♻️ Suggested simplification
remotes, err := getRemotes(ctx, repoPath) if err == nil { repo.Remotes = remotes - } else if errors.Is(err, context.Canceled) { - return Repository{}, err } if err := ctx.Err(); err != nil { return Repository{}, err } dirty, err := isDirty(ctx, repoPath) if err == nil { repo.IsDirty = dirty - } else if errors.Is(err, context.Canceled) { - return Repository{}, err }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/git/scanner.go` around lines 163 - 179, The checks for errors.Is(err, context.Canceled) after calling getRemotes and isDirty are dead/unreliable for killed subprocesses; remove those branches and rely on the existing ctx.Err() guard instead. Specifically, in internal/git/scanner.go update the getRemotes and isDirty call sites (the variables remotes, err := getRemotes(ctx, repoPath) and dirty, err := isDirty(ctx, repoPath)) to drop the else if errors.Is(err, context.Canceled) return Repository{}, err blocks and simply handle success by assigning repo.Remotes / repo.IsDirty and otherwise let the subsequent ctx.Err() check and the generic error return handle cancellation and other errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@cmd/root.go`:
- Around line 552-564: The bridge goroutine that reads scanChan and calls
saveRegistry(reg, regPath) is not awaited: current code only waits on <-scanDone
(scanner) so the program can exit before the bridge finishes persisting registry
changes. Fix by adding an explicit synchronization for the bridge (e.g., a done
channel or sync.WaitGroup) that the bridge signals after saveRegistry returns,
then wait for that signal before returning/exiting (on the ui.ErrCancelled path
and the normal shutdown path); reference the bridge goroutine that closes over
scanChan and calls saveRegistry(reg, regPath), the existing scanDone waiter, and
the cancelScan() call to ensure the new wait happens after cancelScan() and
draining.
In `@internal/ui/repo_selector.go`:
- Around line 1226-1231: Remove the unused local helper function min in
internal/ui/repo_selector.go: delete the min(a, b int) int definition since
golangci-lint flags it as unused and Go 1.21+ provides a built-in min; also
search for any residual callers (e.g., inside resolveRainBackgroundWidth or
RainDisplayWidth) and replace them with the language built-in min if any remain,
ensuring no references to the min function are left behind.
---
Nitpick comments:
In `@internal/git/scanner.go`:
- Around line 163-179: The checks for errors.Is(err, context.Canceled) after
calling getRemotes and isDirty are dead/unreliable for killed subprocesses;
remove those branches and rely on the existing ctx.Err() guard instead.
Specifically, in internal/git/scanner.go update the getRemotes and isDirty call
sites (the variables remotes, err := getRemotes(ctx, repoPath) and dirty, err :=
isDirty(ctx, repoPath)) to drop the else if errors.Is(err, context.Canceled)
return Repository{}, err blocks and simply handle success by assigning
repo.Remotes / repo.IsDirty and otherwise let the subsequent ctx.Err() check and
the generic error return handle cancellation and other errors.
In `@internal/ui/path_display.go`:
- Around line 84-89: The hard-coded reserve = 34 is undocumented and hides the
width contributions from the per-row dirty indicator and the focused-row scroll
hint; update the pw calculation in path_display.go to replace the magic number
with a named constant (e.g., scrollHintWidth or reservedWidth) or add a clear
inline comment explaining the derivation (dirty indicator ~3 cells + scroll hint
~19 cells + any padding = 34) and use that constant when computing pw alongside
prefixW and suffixW so future changes to the dirty indicator or scroll hint are
obvious; reference the existing variables prefixW, suffixW, reserve (or new
constant) and pw to locate where to apply the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 139853fb-5261-473b-8d30-5b4d28edcc8d
📒 Files selected for processing (10)
CLAUDE.mdREADME.mdcmd/root.gointernal/git/scanner.gointernal/git/scanner_test.gointernal/ui/config_view.gointernal/ui/panel_layout.gointernal/ui/panel_layout_test.gointernal/ui/path_display.gointernal/ui/repo_selector.go
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/ui/config_view.go
- README.md
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Missing space between label and value in settings view
- Added a literal space in the format string between the styled label and value so rows render as "label: value" again.
Preview (c116659eb1)
diff --git a/CLAUDE.md b/CLAUDE.md
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -9,10 +9,8 @@
1. **Default** — `git fetch --all` per repo. `--prune` is opt-in. Precedence per repo: CLI (`--prune` / `--no-prune`) → local `git config rain.fetchprune` → registry `fetch_prune` → global `fetch_prune`.
2. **`--fetch-mainline`** — targeted fetches for mainline branches only (incompatible with `--sync` / full-sync triggers; CLI returns an error if combined).
3. **`--sync` + `branch_mode`** — hydrate local branches from remotes.
-4. **`--risky`** — on the sync path only; allows hard reset to upstream after backup refs.
+4. **`--risky`** — on the full branch-hydration path; allows hard reset to upstream after backup refs.
-Extracted from the `git-fire` codebase and promoted to a first-class tool.
-
Module: `github.com/git-rain/git-rain`
Go version: 1.24.2
Config: `~/.config/git-rain/config.toml`
@@ -58,8 +56,8 @@
**Key design decisions:**
- Uses native `git` binary via `exec.Command` — not go-git.
- Default run: `git fetch --all` with optional `--prune` (resolved per repo: CLI → `rain.fetchprune` → registry `fetch_prune` → `global.fetch_prune`) and optional `--tags`. Mainline-only: `internal/git.MainlineFetchRemotes` when `--fetch-mainline`. Local hydrate: `RainRepository` when `--sync`, non-mainline `branch_mode`, or risky-only config forces full sync.
-- Interactive picker: `--rain` (mirrors `git-fire --fire`).
-- Backup branch prefix: `git-rain-backup-` (was `git-fire-rain-backup-` in git-fire).
+- Interactive picker: `--rain` (streaming scan + Bubble Tea UI). Panel width math lives in `internal/ui/panel_layout.go` (`PanelBlockWidth` / `PanelTextWidth`); keep it aligned with `boxStyle` horizontal padding. On exit, `runRainTUIStream` cancels the scan context before draining channels so in-flight `git` from `ScanRepositoriesStream` can abort; OS SIGINT maps to the same cancel path as `q` via `tea.ErrInterrupted` → `ErrCancelled`.
+- Backup branch prefix: `git-rain-backup-`.
- Config env prefix: `GIT_RAIN_`.
- Safe mode (default): never rewrites local-only commits (applies to `--sync` path).
- Risky mode (`--risky` / `config: global.risky_mode`): allows hard reset to upstream after creating a `git-rain-backup-*` ref (implies full sync).
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -18,10 +18,26 @@
git-rain → fetch all remotes by default, or hydrate locals with --sync-git-rain discovers git repositories under your scan path (and known registry entries). Default: git fetch --all per repo (no --prune unless you opt in) so remote-tracking refs update without moving local branches. Lighter fetch: --fetch-mainline. Local updates: --sync with --branch-mode / config. Destructive realignment: --risky on the sync path only.
+git-rain discovers git repositories under your scan path (and known registry entries). From lightest to heaviest:
-> Warning: --prune is opt-in. Passing --prune on git fetch deletes stale remote-tracking branch refs (for example refs/remotes/origin/old-feature after that branch was removed on the server). That is usually what you want for a tidy clone, but it removes those ref names locally until the next fetch brings them back if the branch reappears. Enable pruning only when you intend it, in this order of override: --prune / --no-prune for this run, per-repo git config --local --bool rain.fetchprune, fetch_prune on a registry entry, or global.fetch_prune in config. Effective precedence for each repo is: CLI → rain.fetchprune → registry fetch_prune → global fetch_prune.
+| Mode | What it does |
+| --- | --- |
+| Default | git fetch --all per repo. --prune is opt-in (see below). Updates remote-tracking refs; does not move local branches. |
+| Lighter fetch | --fetch-mainline — mainline remote-tracking refs only. |
+| Local updates | --sync — hydrate locals; scope from --branch-mode or config. |
+| Destructive realignment | --risky or config risky_mode on the full branch-hydration path (same machinery as --sync). That path also runs without --sync when you pass --risky, set risky_mode, use a non-mainline branch_mode in config, or pass any --branch-mode value on the CLI (even mainline). Hard-reset to upstream only after backup refs. |
+> Warning: --prune is opt-in. On git fetch, --prune deletes stale remote-tracking branch refs (for example refs/remotes/origin/old-feature after that branch was removed on the server). That is usually what you want for a tidy clone, but it removes those ref names locally until a later fetch brings them back if the branch reappears. Turn pruning on only when you mean to.
+
+Where --prune is decided (per repo, first applicable source wins):
+
+1. CLI — --prune or --no-prune for this run
+2. Local git config — git config --local --bool rain.fetchprune
+3. Registry — fetch_prune on the repo entry
+4. User config — global.fetch_prune
+
+Precedence chain: CLI → rain.fetchprune → registry fetch_prune → global fetch_prune.
+
Invocation note: git-rain and git rain are equivalent when git-rain is on your PATH.
Table of Contents
@@ -51,7 +67,8 @@
Quick Start
-# preview first — shows what would be synced without touching anything
+# preview first — lists repos and whether each would get fetch-only vs branch hydration, without running git
+# (still does a filesystem scan; the flag name is a little ironic — "dry" rain that still kicks up dust)
git-rain --dry-run
# default: scan repos, then git fetch --all per repo (no --prune unless configured or --prune)
@@ -167,7 +184,7 @@
4. **`--sync`** — hydrates **local** branches: `git fetch --all` (same prune/tags rules), then updates eligible locals toward upstream. Scope comes from **`--branch-mode`** or config `branch_mode`: `mainline`, `checked-out`, `all-local`, or **`all-branches`** (creates local tracking branches for remotes you do not have yet — can be many branches).
-5. **`--risky`** — does not change fetch behavior by itself; on the **`--sync`** path it allows hard reset to upstream after creating `git-rain-backup-*` refs when you would otherwise skip local-only commits.
+5. **`--risky`** — does not change fetch behavior by itself; on the **full branch-hydration** path (see `--sync` above — entered by `--sync`, **`--risky`**, config **`risky_mode`**, a **non-mainline `branch_mode`**, or **any `--branch-mode` flag** on the CLI) it allows hard reset to upstream after creating `git-rain-backup-*` refs when you would otherwise skip local-only commits.
6. **Report** — one summary line per repo on the default full fetch; per-branch lines on `--fetch-mainline` and `--sync`.
@@ -177,7 +194,7 @@
- **Safety-first defaults** — never rewrites local-only commits; dirty worktrees are skipped, not clobbered
- **Risky mode** — opt-in destructive realignment: creates a `git-rain-backup-*` ref, then hard-resets to upstream
- **Non-checked-out branches** — updated directly without touching the worktree
-- **Interactive TUI (`--rain`)** — streaming repo picker (mirrors `git-fire --fire`), then the same default fetch, `--fetch-mainline`, or `--sync` behavior
+- **Interactive TUI (`--rain`)** — streaming repo picker, then the same default fetch, `--fetch-mainline`, or `--sync` behavior
- **Registry** — discovered repos persist across runs; mark repos ignored to skip them permanently
- **Dry run** — preview all repos that would be fetched or synced without making any changes
- **`--fetch-mainline`** — mainline-only remote-tracking ref refresh instead of the default full `git fetch --all`
@@ -220,8 +237,8 @@
| Flag | Description |
|---|---|
-| `--dry-run` | Show what would run without making changes |
-| `--rain` | Interactive TUI repo picker before running (like `git-fire --fire`) |
+| `--dry-run` | No `git fetch` / branch updates — still scans disk to list repos. The name is weather-themed irony: no “wet” git work, but not a no-op. |
+| `--rain` | Interactive TUI repo picker before running |
| `--sync` | Update local branches from remotes (after `git fetch --all`; default run does not sync locals) |
| `--fetch-mainline` | Mainline-only remote `git fetch` per remote instead of default `git fetch --all` (not with `--sync` or other full-sync triggers) |
| `--branch-mode` | With `--sync`: `mainline`, `checked-out`, `all-local`, or `all-branches` (overrides config for this run) |
@@ -251,13 +268,17 @@
```toml
[global]
scan_path = "/home/you/projects" # root to discover repos under
-scan_depth = 5 # max directory depth
-scan_workers = 8 # parallel scan workers
-risky_mode = false # destructive realignment on --sync path only
-branch_mode = "mainline" # used with --sync: mainline | checked-out | all-local | all-branches
-fetch_prune = false # pass --prune on fetch when true (default off; see README warning)
-default_mode = "safe" # "safe" or "risky"
-disable_scan = false # skip scan; use registry only
+scan_depth = 5 # max directory depth (default in app: 10)
+scan_workers = 8 # parallel scan workers
+fetch_workers = 4 # parallel per-repo operations (default in app: 4)
+risky_mode = false # allow destructive realignment on full hydration path
+branch_mode = "mainline" # full hydration: mainline | checked-out | all-local | all-branches
+fetch_prune = false # pass --prune on fetch when true (default off; see README warning)
+sync_tags = false # pass --tags on fetch when true; CLI --tags still forces tags for the run
+# Registry default for new repos (TUI / opt-out): leave-untouched | sync-default | sync-all | sync-current-branch
+default_mode = "sync-default"
+disable_scan = false # skip scan; use registry only
+mainline_patterns = [] # extra mainline names/prefixes when branch_mode = mainline
scan_exclude = [
"node_modules",
@@ -266,6 +287,8 @@
]+global.default_mode must be exactly one of the four values listed above; anything else fails config load.
+
All options can be overridden with environment variables using the GIT_RAIN_ prefix:
@@ -273,9 +296,15 @@
GIT_RAIN_GLOBAL_SCAN_PATH=/tmp/repos git-rain+### Config file, locks, and crashes
+
+Registry (repos.toml) — Writes use a cross-process lock file (repos.toml.lock), atomic replace, and stale-lock detection (owner PID). If a process dies mid-run you may still see a leftover lock: the CLI prompts to remove it when safe, or you can use --force-unlock-registry in scripts. This is the same class of “stale lock / don’t corrupt the database” problem as other multi-repo tools; treat lock removal like any other forced unlock — only when you are sure no other git-rain is running.
+
+User config (config.toml) — There is no cross-process lock on the config file today. The TUI saves settings with an atomic write (temp file then rename into place), so an ungraceful exit mid-save should not replace config.toml with a half-written file; you might leave an orphan config.toml.tmp, which is safe to delete if present. Avoid hand-editing config.toml at the same moment an interactive --rain session is saving, or two editors racing writes — a future shared lock could remove that race.
+
Interactive TUI
-git-rain --rain opens an interactive picker. Repositories stream in as the filesystem scan finds them — no waiting for the full scan to complete before you can start picking. After you confirm, the tool runs the default full fetch (git fetch --all, prune opt-in) unless you passed --fetch-mainline, or --sync / config implies full branch hydration.
+git-rain --rain opens an interactive picker. Repositories stream in as the filesystem scan finds them — no waiting for the full scan to complete before you can start picking. After you confirm, the tool runs the default full fetch (git fetch --all, prune opt-in) unless you passed --fetch-mainline, or full branch hydration is implied by --sync, --risky, risky_mode in config, a non-mainline branch_mode, or any --branch-mode on the CLI. Quitting (q or ctrl+c) cancels the in-progress scan (in-flight git subprocesses are aborted via the scan context); ctrl+c outside raw TTY mode is treated like cancel.
Key bindings:
@@ -284,7 +313,8 @@
| space | Toggle repo selection |
| a | Select all / deselect all |
| enter | Confirm selection and begin fetch or sync |
-| q / esc | Abort |
+| q / ctrl+c | Abort picker |
+| c / Esc | Back from settings (ignored list uses Esc / i / b) |
| ↑ / ↓ | Navigate |
Safe Mode vs Risky Mode
@@ -296,7 +326,7 @@
| Checked-out branch, dirty worktree | ⊘ Skipped | ⊘ Skipped |
| No upstream tracked | ⊘ Skipped | ⊘ Skipped |
-In risky mode, a git-rain-backup-<branch>-<timestamp> ref is created before any hard reset so local work is always recoverable.
+In risky mode, a backup ref named like git-rain-backup-<sanitized-branch>-<timestamp>-<short-sha> is created before any hard reset so local work is always recoverable.
Registry
diff --git a/cmd/root.go b/cmd/root.go
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -53,7 +53,8 @@
use --prune, config fetch_prune, registry fetch_prune, or git config rain.fetchprune.
- --fetch-mainline — targeted fetches for mainline branches only (faster when
-
you do not need every remote ref).
-
you do not need every remote ref). Cannot be combined with --sync, --risky, -
non-mainline --branch-mode, or global risky_mode (full-sync triggers).- --sync — hydrate local branches from remotes. Use --branch-mode for scope
(mainline, checked-out, all-local, all-branches); all-branches can create
@@ -91,7 +92,7 @@
rootCmd.Flags().BoolVar(&rainFetchMainline, "fetch-mainline", false, "Fetch only mainline remote-tracking refs per remote (lighter than the default git fetch --all)")
rootCmd.Flags().BoolVar(&rainInit, "init", false, "Generate example ~/.config/git-rain/config.toml")
rootCmd.Flags().StringVar(&rainConfigFile, "config", "", "Use an explicit config file path")
- --sync — hydrate local branches from remotes. Use --branch-mode for scope
- rootCmd.Flags().BoolVar(&rainRain, "rain", false, "Interactive TUI repo picker before running (mirrors git-fire --fire)")
-
rootCmd.Flags().BoolVar(&rainRain, "rain", false, "Interactive TUI repo picker before running")
rootCmd.Flags().BoolVar(&rainSync, "sync", false, "Update local branches from remotes (default is git fetch --all only)")
rootCmd.Flags().StringVar(&rainBranchMode, "branch-mode", "",Branch sync mode: mainline (default), checked-out, all-local, all-branches)
rootCmd.Flags().BoolVar(&rainSyncTags, "tags", false, "Fetch all tags from remotes (default: off)")
@@ -510,7 +511,8 @@
opts.FolderProgress = folderProgressscanChan := make(chan git.Repository, opts.Workers)
- tuiRepoChan := make(chan git.Repository, opts.Workers)
-
// Large buffer so the bridge rarely blocks on send if the TUI stops consuming.
-
tuiRepoChan := make(chan git.Repository, 256)
var scanErr error
scanDone := make(chan struct{})
@@ -547,7 +549,10 @@
regPath,
)
- // Drain channels before cancelling so goroutines can't block on sends.
- // Cancel scan first so filepath walk and in-flight git subprocesses unwind.
- cancelScan()
- // Drain the TUI channel so the bridge never blocks on send while the scanner closes.
go func() {
for range tuiRepoChan {
}
@@ -556,7 +561,6 @@
for range folderProgress {
}
}()
-
cancelScan()
<-scanDoneif err != nil {
diff --git a/cmd/root_test.go b/cmd/root_test.go
--- a/cmd/root_test.go
+++ b/cmd/root_test.go
@@ -102,6 +102,36 @@
}
}
+func TestRunRain_DryRunSyncShowsPruneResolution(t *testing.T) {
- tmpHome := t.TempDir()
- setTestUserDirs(t, tmpHome)
- scenario := testutil.NewScenario(t)
- repo := scenario.CreateRepo("dry-sync-prune").
-
AddFile("a.txt", "x\n"). -
Commit("init") - resetFlags()
- rainPath = filepath.Dir(repo.Path())
- rainDryRun = true
- rainSync = true
- rainPrune = true
- var runErr error
- output := captureStdout(t, func() {
-
runErr = runRain(rootCmd, []string{}) - })
- if runErr != nil {
-
t.Fatalf("runRain() dry-run sync error = %v", runErr) - }
- if !strings.Contains(output, "Fetch --prune: on for this run (--prune)") {
-
t.Fatalf("expected dry-run to show prune on for --sync path, got:\n%s", output) - }
- if !strings.Contains(output, "Would hydrate") {
-
t.Fatalf("expected dry-run hydrate wording for --sync, got:\n%s", output) - }
+}
func TestRunRain_DefaultFetchAllDoesNotMoveLocalBranch(t *testing.T) {
tmpHome := t.TempDir()
setTestUserDirs(t, tmpHome)
diff --git a/go.mod b/go.mod
--- a/go.mod
+++ b/go.mod
@@ -26,6 +26,7 @@
github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect
github.com/fsnotify/fsnotify v1.9.0 // indirect
github.com/go-viper/mapstructure/v2 v2.4.0 // indirect
- github.com/gofrs/flock v0.12.1 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/lucasb-eyer/go-colorful v1.3.0 // indirect
github.com/mattn/go-localereader v0.0.1 // indirect
diff --git a/go.sum b/go.sum
--- a/go.sum
+++ b/go.sum
@@ -33,6 +33,8 @@
github.com/git-fire/git-testkit v0.2.0/go.mod h1:YlJlkY9JfGdYTe9o9W3l+gv9BPj05FGu6HK36Z5jwVA=
github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs=
github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM=
+github.com/gofrs/flock v0.12.1 h1:MTLVXXHf8ekldpJk3AKicLij9MdwOWkZ+a/jHHZby9E=
+github.com/gofrs/flock v0.12.1/go.mod h1:9zxTsyu5xtJ9DK+1tFZyibEV7y3uwDxPPfbxeeHCoD0=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
@@ -98,5 +100,6 @@
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -3,6 +3,7 @@
import (
"os"
"path/filepath"
-
"sync"
"testing""github.com/git-rain/git-rain/internal/config"
@@ -109,6 +110,37 @@
}
}
+func TestSaveConfig_ConcurrentWrites(t *testing.T) {
- dir := t.TempDir()
- cfgPath := filepath.Join(dir, "config.toml")
- var wg sync.WaitGroup
- for i := 0; i < 8; i++ {
-
wg.Add(1) -
go func(i int) { -
defer wg.Done() -
c := config.DefaultConfig() -
if i%2 == 0 { -
c.Global.BranchMode = "mainline" -
} else { -
c.Global.BranchMode = "all-local" -
} -
if err := config.SaveConfig(&c, cfgPath); err != nil { -
t.Errorf("SaveConfig %d: %v", i, err) -
} -
}(i) - }
- wg.Wait()
- loaded, err := config.LoadWithOptions(config.LoadOptions{ConfigFile: cfgPath})
- if err != nil {
-
t.Fatalf("Load after concurrent saves: %v", err) - }
- if loaded.Global.BranchMode != "mainline" && loaded.Global.BranchMode != "all-local" {
-
t.Fatalf("unexpected branch mode %q", loaded.Global.BranchMode) - }
+}
func TestSaveConfig_RoundTrip(t *testing.T) {
dir := t.TempDir()
cfgPath := filepath.Join(dir, "config.toml")
@@ -148,11 +180,30 @@
}
}
+func TestValidate_DefaultMode_EmptyBecomesDefault(t *testing.T) {
- cfg := config.DefaultConfig()
- cfg.Global.DefaultMode = ""
- if err := cfg.Validate(); err != nil {
-
t.Fatalf("Validate() error = %v", err) - }
- if cfg.Global.DefaultMode != "sync-default" {
-
t.Fatalf("DefaultMode = %q, want sync-default", cfg.Global.DefaultMode) - }
+}
+func TestValidate_DefaultMode_Invalid(t *testing.T) {
- cfg := config.DefaultConfig()
- cfg.Global.DefaultMode = "push-known-branches"
- if err := cfg.Validate(); err == nil {
-
t.Fatal("Validate() should reject invalid default_mode") - }
+}
func TestExampleConfigTOML_ContainsBranchMode(t *testing.T) {
toml := config.ExampleConfigTOML()
- for _, want := range []string{"branch_mode", "sync_tags", "fetch_prune", "mainline_patterns"} {
- for _, want := range []string{"branch_mode", "sync_tags", "fetch_prune", "mainline_patterns",
default_mode = "sync-default"} {
if !contains(toml, want) {
-
t.Errorf("ExampleConfigTOML missing key %q", want)
-
}
t.Errorf("ExampleConfigTOML missing fragment %q", want) }
}
diff --git a/internal/config/defaults.go b/internal/config/defaults.go
--- a/internal/config/defaults.go
+++ b/internal/config/defaults.go
@@ -77,7 +77,7 @@
fetch_workers = 4
Default mode for repos (used by registry opt-out model)
-default_mode = "push-known-branches"
+default_mode = "sync-default"
Re-scan known repos for new submodules
rescan_submodules = false
diff --git a/internal/config/loader.go b/internal/config/loader.go
--- a/internal/config/loader.go
+++ b/internal/config/loader.go
@@ -6,6 +6,7 @@
"path/filepath"
"strings"
- "github.com/gofrs/flock"
"github.com/pelletier/go-toml/v2"
"github.com/spf13/viper"
)
@@ -97,17 +98,24 @@
}
// SaveConfig writes cfg to path as TOML. Intermediate directories are created
-// if needed. Existing file content is replaced atomically.
+// if needed. Uses an exclusive lock (path + ".lock") and a PID-scoped temp file
+// so concurrent writers or interrupted renames cannot corrupt the live config.
func SaveConfig(cfg *Config, path string) error {
dir := filepath.Dir(path)
if err := os.MkdirAll(dir, 0o700); err != nil {
return fmt.Errorf("creating config directory: %w", err)
}
- lock := flock.New(path + ".lock")
- if err := lock.Lock(); err != nil {
-
return fmt.Errorf("config file lock: %w", err) - }
- defer func() { _ = lock.Unlock() }()
- data, err := toml.Marshal(cfg)
if err != nil {
return fmt.Errorf("marshalling config: %w", err)
}
- tmp := path + ".tmp"
- tmp := fmt.Sprintf("%s.%d.tmp", path, os.Getpid())
if err := os.WriteFile(tmp, data, 0o600); err != nil {
return fmt.Errorf("writing temp config: %w", err)
}
@@ -130,6 +138,16 @@
// Validate checks if the configuration is valid.
func (c *Config) Validate() error {
- dm := strings.TrimSpace(c.Global.DefaultMode)
- if dm == "" {
-
dm = DefaultConfig().Global.DefaultMode - }
- switch dm {
- case "leave-untouched", "sync-default", "sync-all", "sync-current-branch":
-
c.Global.DefaultMode = dm - default:
-
return fmt.Errorf("global.default_mode must be one of leave-untouched, sync-default, sync-all, sync-current-branch, got %q", c.Global.DefaultMode) - }
if c.Global.FetchWorkers <= 0 {
c.Global.FetchWorkers = DefaultFetchWorkers
}
@@ -193,14 +211,26 @@
}
// WriteExampleConfig writes an example config file to the specified path.
+// Same locking and atomic replace semantics as SaveConfig.
func WriteExampleConfig(path string) error {
dir := filepath.Dir(path)
if err := os.MkdirAll(dir, 0o700); err != nil {
return fmt.Errorf("failed to create config directory: %w", err)
}
- lock := flock.New(path + ".lock")
- if err := lock.Lock(); err != nil {
-
return fmt.Errorf("config file lock: %w", err) - }
- defer func() { _ = lock.Unlock() }()
- content := ExampleConfigTOML()
- if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
-
return fmt.Errorf("failed to write config file: %w", err)
- tmp := fmt.Sprintf("%s.%d.tmp", path, os.Getpid())
- if err := os.WriteFile(tmp, []byte(content), 0o600); err != nil {
-
}
return fmt.Errorf("failed to write temp config: %w", err) - if err := os.Rename(tmp, path); err != nil {
-
_ = os.Remove(tmp) -
return fmt.Errorf("failed to replace config file: %w", err) - }
return nil
}
diff --git a/internal/git/scanner.go b/internal/git/scanner.go
--- a/internal/git/scanner.go
+++ b/internal/git/scanner.go
@@ -2,6 +2,7 @@
import (
"context"
- "errors"
"fmt"
"os"
"os/exec"
@@ -46,7 +47,7 @@
if err != nil || !fi.IsDir() {
return
}
-
repo, err := analyzeRepository(p)
-
repo, err := analyzeRepository(ctx, p) if err != nil { return }
@@ -146,31 +147,42 @@
// AnalyzeRepository extracts metadata from a git repository at repoPath.
func AnalyzeRepository(repoPath string) (Repository, error) {
- return analyzeRepository(repoPath)
- return analyzeRepository(context.Background(), repoPath)
}
-func analyzeRepository(repoPath string) (Repository, error) {
+func analyzeRepository(ctx context.Context, repoPath string) (Repository, error) {
- if err := ctx.Err(); err != nil {
-
return Repository{}, err - }
repo := Repository{
Path: repoPath,
Name: filepath.Base(repoPath),
Selected: true,
}
- remotes, err := getRemotes(repoPath)
- remotes, err := getRemotes(ctx, repoPath)
if err == nil {
repo.Remotes = remotes - } else if errors.Is(err, context.Canceled) {
-
}
return Repository{}, err
- dirty, err := isDirty(repoPath)
-
if err := ctx.Err(); err != nil {
-
return Repository{}, err -
}
-
dirty, err := isDirty(ctx, repoPath)
if err == nil {
repo.IsDirty = dirty -
} else if errors.Is(err, context.Canceled) {
-
return Repository{}, err}
return repo, nil
}
-func getRemotes(repoPath string) ([]Remote, error) {
- cmd := exec.Command("git", "remote", "-v")
+func getRemotes(ctx context.Context, repoPath string) ([]Remote, error) {
-
cmd := exec.CommandContext(ctx, "git", "remote", "-v")
cmd.Dir = repoPathoutput, err := cmd.Output()
@@ -210,8 +222,8 @@
return remotes, nil
}
-func isDirty(repoPath string) (bool, error) {
- cmd := exec.Command("git", "status", "--porcelain")
+func isDirty(ctx context.Context, repoPath string) (bool, error) {
-
cmd := exec.CommandContext(ctx, "git", "status", "--porcelain")
cmd.Dir = repoPathoutput, err := cmd.Output()
diff --git a/internal/git/scanner_test.go b/internal/git/scanner_test.go
new file mode 100644
--- /dev/null
+++ b/internal/git/scanner_test.go
@@ -1,0 +1,41 @@
+package git
+
+import (
- "context"
- "testing"
- testutil "github.com/git-fire/git-testkit"
+)
+// TestScanRepositoriesStream_PreCancelledDrain verifies that when the scan
+// context is already cancelled, ScanRepositoriesStream still closes the output
+// channel so callers never block on range. This guards the --rain quit path
+// where cancelScan runs while the picker exits.
+func TestScanRepositoriesStream_PreCancelledDrain(t *testing.T) {
- scenario := testutil.NewScenario(t)
- repo := scenario.CreateRepo("scan-pre-cancel").
-
AddFile("README.md", "x\n"). -
Commit("init") - ctx, cancel := context.WithCancel(context.Background())
- cancel()
- out := make(chan Repository, 8)
- opts := ScanOptions{
-
RootPath: repo.Path(), -
Workers: 2, -
Ctx: ctx, -
MaxDepth: 4, -
DisableScan: true, -
KnownPaths: map[string]bool{repo.Path(): false}, -
FolderProgress: nil, - }
- err := ScanRepositoriesStream(opts, out)
- if err != nil {
-
t.Fatalf("ScanRepositoriesStream: %v", err) - }
- for range out {
- }
+}
diff --git a/internal/registry/types.go b/internal/registry/types.go
--- a/internal/registry/types.go
+++ b/internal/registry/types.go
@@ -21,7 +21,7 @@
// Status: "active", "missing", or "ignored"
Status string toml:"status"
- // Last-used mode (e.g. "push-known-branches")
-
// Last-used per-repo disposition: leave-untouched | sync-default | sync-all | sync-current-branch
Mode stringtoml:"mode,omitempty"// Per-repo override for submodule re-scanning.
diff --git a/internal/ui/config_view.go b/internal/ui/config_view.go
--- a/internal/ui/config_view.go
+++ b/internal/ui/config_view.go
@@ -27,10 +27,10 @@
var configRows = []configRow{
{label: "Default mode", kind: configRowEnum, options: []string{
-
"push-known-branches", -
"push-all",
-
"sync-default", -
"sync-all", -
"sync-current-branch", "leave-untouched",
-
}},
"push-current-branch",
{label: "Disable scan", kind: configRowBool},
{label: "Fetch workers", kind: configRowEnum, options: []string{
@@ -244,10 +244,10 @@
func (m RepoSelectorModel) viewConfig() string {
var s strings.Builder
-
cw := m.contentWidth()
if m.rainVisible() {
-
cw := m.contentWidth() -
rainW := min(cw, 70)
-
rainW := RainDisplayWidth(m.windowWidth) s.WriteString(m.rainBg.Render()) s.WriteString("\n") s.WriteString(RenderRainWave(rainW, m.frameIndex, m.rainAnimationMode))
@@ -259,7 +259,12 @@
Foreground(activeProfile().titleFg).
Background(activeProfile().titleBg).
Padding(0, 2)
- s.WriteString(titleGradient.Render("🌧️ GIT RAIN — SETTINGS"))
-
title := "🌧️ GIT RAIN — SETTINGS"
-
if cw <= 0 {
-
s.WriteString(titleGradient.Render(title)) -
} else {
-
s.WriteString(titleGradient.MaxWidth(cw).Render(title)) -
}
s.WriteString("\n\n")cursorStyle := lipgloss.NewStyle().Foreground(activeProfile().configCursor).Bold(true)
@@ -283,29 +288,32 @@
case configRowComingSoon:
hintStr = dimStyle.Render(" coming soon")
default:
-
hintStr = dimStyle.Render(" ←/→ to change")
-
if cw >= 88 { -
hintStr = dimStyle.Render(" ←/→ to change") -
} else if cw >= 64 { -
hintStr = dimStyle.Render(" ←/→") -
} } }
-
line := fmt.Sprintf("%s %-32s %s%s",
-
line := fmt.Sprintf("%s %s %s%s", cursorStyle.Render(cur), labelStyle.Render(row.label+":"), valueStyle.Render(val), hintStr, )
-
s.WriteString(line)
-
s.WriteString(clampCellWidth(line, cw)) s.WriteString("\n")}
s.WriteString("\n")
if m.configSaveErr != nil {
errStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("#FF6666"))
-
s.WriteString(errStyle.Render("⚠️ Save failed: " + m.configSaveErr.Error()))
-
s.WriteString(clampCellWidth(errStyle.Render("⚠️ Save failed: "+m.configSaveErr.Error()), cw)) s.WriteString("\n")
-
s.WriteString(helpStyle.Render( -
"In-memory settings updated; fix the error above to persist to disk.\n" + -
"Controls: ↑/k, ↓/j Navigate | space/→ Next value | ← Prev value | c/Esc Back | q Quit", -
))
-
helpText := "In-memory settings updated; fix the error above to persist to disk.\n" + -
"Controls: ↑/k, ↓/j Navigate | space/→ Next value | ← Prev value | c/Esc Back | q Quit" -
} else {
s.WriteString(helpStyle.MaxWidth(cw).Render(helpText))
cfgPathStr := m.cfgPath
if cfgPathStr == "" {
@@ -313,13 +321,13 @@
} else {
cfgPathStr = AbbreviateUserHome(cfgPathStr)
}
-
s.WriteString(helpStyle.Render( -
"Changes saved immediately to " + cfgPathStr + "\n" + -
"Controls: ↑/k, ↓/j Navigate | space/→ Next value | ← Prev value | c/Esc Back | q Quit", -
))
-
helpText := "Changes saved immediately to " + cfgPathStr + "\n" + -
"Controls: ↑/k, ↓/j Navigate | space/→ Next value | ← Prev value | c/Esc Back | q Quit" -
}
s.WriteString(helpStyle.MaxWidth(cw).Render(helpText))
- return boxStyle.Render(s.String())
- innerW := PanelBlockWidth(m.windowWidth)
- return boxStyle.Width(innerW).Render(s.String())
}
func (m RepoSelectorModel) syncRuntimeFromConfig(cmds []tea.Cmd) (RepoSelectorModel, []tea.Cmd) {
diff --git a/internal/ui/panel_layout.go b/internal/ui/panel_layout.go
new file mode 100644
--- /dev/null
+++ b/internal/ui/panel_layout.go
@@ -1,0 +1,41 @@
+package ui
+
+// Horizontal layout for the main Bubble Tea panel (must stay consistent across
+// repo list, ignored list, settings, rain banner, and PathWidthFor).
+
+// panelOuterMarginTotal is the number of terminal columns reserved outside the
+// bordered panel so the frame does not touch the left/right edge.
+const panelOuterMarginTotal = 6
+
+// panelBoxHorizontalPadding is the sum of left and right padding on boxStyle
+// (Padding(1, 2) => 2 + 2). Keep in sync with boxStyle in repo_selector.go and
+// applyColorProfile in color_profiles.go.
+const panelBoxHorizontalPadding = 4
+
+// PanelBlockWidth returns the lipgloss Width passed to boxStyle for the main panel.
+func PanelBlockWidth(terminalWidth int) int {
- w := terminalWidth - panelOuterMarginTotal
- if w < 0 {
-
return 0 - }
- return w
+}
+// PanelTextWidth is the maximum cell width for one line of content inside the
+// panel after horizontal padding (use for clamping, PathWidthFor, MaxWidth).
+func PanelTextWidth(terminalWidth int) int {
- w := PanelBlockWidth(terminalWidth) - panelBoxHorizontalPadding
- if w < 0 {
-
return 0 - }
- return w
+}
+// RainDisplayWidth caps the ASCII rain banner width to match the text area.
+func RainDisplayWidth(terminalWidth int) int {
- tw := PanelTextWidth(terminalWidth)
- if tw > 70 {
-
return 70 - }
- return tw
+}
diff --git a/internal/ui/panel_layout_test.go b/internal/ui/panel_layout_test.go
new file mode 100644
--- /dev/null
+++ b/internal/ui/panel_layout_test.go
@@ -1,0 +1,37 @@
+package ui
+
+import (
- "testing"
- "github.com/charmbracelet/lipgloss"
+)
+// TestPanelLayoutMatchesBoxStylePadding keeps panelBoxHorizontalPadding in sync
+// with the main panel lipgloss style (Border + Padding(1,2)).
+func TestPanelLayoutMatchesBoxStylePadding(t *testing.T) {
- box := lipgloss.NewStyle().
-
Border(lipgloss.RoundedBorder()). -
Padding(1, 2) - if got := box.GetHorizontalPadding(); got != panelBoxHorizontalPadding {
-
t.Fatalf("lipgloss horizontal padding = %d, panelBoxHorizontalPadding = %d — update one to match", -
got, panelBoxHorizontalPadding) - }
+}
+func TestPanelBlockAndTextWidth(t *testing.T) {
- if got := PanelBlockWidth(120); got != 114 {
-
t.Fatalf("PanelBlockWidth(120) = %d, want 114", got) - }
- if got := PanelTextWidth(120); got != 110 {
-
t.Fatalf("PanelTextWidth(120) = %d, want 110", got)
... diff truncated: showing 800 of 1230 lines
</details>
<sub>You can send follow-ups to the cloud agent <a href="https://cursor.com/agents/bc-6504a74e-b09f-48eb-9de8-70d4c3ec94e6">here</a>.</sub>
</details>
Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
RainDisplayWidth was capped at 70 while panel content used full PanelTextWidth, so the first rows were shorter than body lines and lipgloss rounded borders showed gaps on main menu and settings. Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
Lipgloss measures line width with ansi.StringWidth while many desktop terminals render emoji as width-2, so top/bottom border segments could be shorter than content (gaps at corners). Pre-render inner block with lipgloss Width(innerBlock - horizontalPadding) then apply boxStyle. Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
repoListVisibleCount used a fixed overhead (~5 lines for help) while helpStyle.MaxWidth and the bordered scan status wrap to many more rows, so the bordered panel exceeded window height and the top border scrolled away. Binary-search the largest list capacity whose lipgloss.Height(renderMainPanelBox) fits windowHeight; same for ignored view. Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
clampCellWidth uses Inline(true), which collapsed the multiline scan box into one broken line. Render the scan box then wrap in Width(cw) Align(Left) for a full-width left-aligned scanbar. Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
…ait) - Remove unused min() tripping golangci-lint unused - README: clarify --branch-mode applies on full hydration path, not only --sync - PathWidthFor: rename/document path row reserve constant - analyzeRepository: use ctx.Err() after git errors instead of errors.Is Canceled - config: writeAtomicReplacing removes temp file on WriteFile failure; dedupe Save/WriteExample - runRainTUIStream: wait for registry bridge goroutine after scan (saveRegistry) Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
internal/ui/view_layout_test.go (2)
27-27: Nit:capshadows the built-in.Using
capas a local variable name is legal but hides Go’s built-incap().capacity(orbest) reads a little cleaner and matches the naming inview_layout.go.Also applies to: 56-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/ui/view_layout_test.go` at line 27, Rename the local variable named cap (which shadows Go's built-in cap()) to a clearer name like capacity or best in the test function(s) that call mainViewMeasuredRepoListCapacity() (the occurrences around the calls at the current locations and the other instance noted around line 56); update all usages in those scopes to use the new name to avoid hiding the built-in and to match the naming used in view_layout.go.
10-70: Consider a table-driven form for the two capacity tests.Both tests assert the same invariant (
panelOuterHeight(cap) <= windowHeightand maximality atcap+1) against two different view modes. Factoring the fits+maximality check into a helper and driving it from a small table would share the assertion logic and make future views trivial to add. Keeping as-is is fine for now.As per coding guidelines: "Prefer table-driven tests for multi-case functions".
♻️ Sketch
type capCase struct { name string windowW int windowH int listLen int measure func(m RepoSelectorModel) int outerH func(m RepoSelectorModel, cap int) int buildModel func(n int) RepoSelectorModel } // iterate cases, run the two assertions once🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/ui/view_layout_test.go` around lines 10 - 70, Refactor the two tests TestMainViewMeasuredRepoListCapacityFitsWindow and TestIgnoredMeasuredListCapacityFitsWindow into a table-driven test that iterates cases; for each case build the RepoSelectorModel (use buildModel), call the appropriate measure function (mainViewMeasuredRepoListCapacity or ignoredMeasuredListCapacity) and assert the same invariants using a shared helper: check capacity >= 1, verify outer height (mainViewPanelOuterHeight or ignoredViewPanelOuterHeight) for cap fits within windowHeight, and verify cap+1 does not fit; reference the functions mainViewMeasuredRepoListCapacity, ignoredMeasuredListCapacity, mainViewPanelOuterHeight and ignoredViewPanelOuterHeight when wiring the case funcs so adding new view modes only requires adding another table entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/ui/repo_selector.go`:
- Around line 715-721: The guard in clampCellWidth currently returns the
original string when maxCells == 1, preventing a 1-cell clamp; change the
early-return condition from "maxCells <= 1" to "maxCells <= 0" (or "maxCells <
1") so that a request to clamp to one cell actually applies lipgloss truncation,
and add a brief comment above clampCellWidth explaining the degeneracy rule
(only non-positive maxCells bypass clamping) so intent is clear; keep the use of
lipgloss.NewStyle().MaxWidth(maxCells).Inline(true).Render(s) unchanged.
---
Nitpick comments:
In `@internal/ui/view_layout_test.go`:
- Line 27: Rename the local variable named cap (which shadows Go's built-in
cap()) to a clearer name like capacity or best in the test function(s) that call
mainViewMeasuredRepoListCapacity() (the occurrences around the calls at the
current locations and the other instance noted around line 56); update all
usages in those scopes to use the new name to avoid hiding the built-in and to
match the naming used in view_layout.go.
- Around line 10-70: Refactor the two tests
TestMainViewMeasuredRepoListCapacityFitsWindow and
TestIgnoredMeasuredListCapacityFitsWindow into a table-driven test that iterates
cases; for each case build the RepoSelectorModel (use buildModel), call the
appropriate measure function (mainViewMeasuredRepoListCapacity or
ignoredMeasuredListCapacity) and assert the same invariants using a shared
helper: check capacity >= 1, verify outer height (mainViewPanelOuterHeight or
ignoredViewPanelOuterHeight) for cap fits within windowHeight, and verify cap+1
does not fit; reference the functions mainViewMeasuredRepoListCapacity,
ignoredMeasuredListCapacity, mainViewPanelOuterHeight and
ignoredViewPanelOuterHeight when wiring the case funcs so adding new view modes
only requires adding another table entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77b80dd0-89ef-4608-adc2-604a915cd797
📒 Files selected for processing (11)
README.mdcmd/root.gointernal/config/loader.gointernal/git/scanner.gointernal/ui/config_view.gointernal/ui/panel_layout.gointernal/ui/panel_layout_test.gointernal/ui/path_display.gointernal/ui/repo_selector.gointernal/ui/view_layout.gointernal/ui/view_layout_test.go
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (6)
- internal/config/loader.go
- internal/ui/panel_layout_test.go
- internal/git/scanner.go
- internal/ui/config_view.go
- internal/ui/panel_layout.go
- internal/ui/path_display.go
…s gap) - README: --dry-run + --no-scan caveat in flags table; dry-run example comment - README: document config.toml.lock + bounded TryLock + atomic write (was wrong) - config: TryLockContext with timeout for SaveConfig/WriteExampleConfig (TUI) - settings: label 'key: ' + comment so value never abuts colon (Bugbot) Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
…ls==1 - clampCellWidth: maxCells < 1 bypass only; document degeneracy (CodeRabbit) - view_layout_test: table-driven cases, helper, rename cap -> capacity Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/ui/repo_selector.go (1)
864-878: Minor: folder truncation slices by bytes, not runes.
folder[len(folder)-maxLen+3:]is a byte slice; for paths containing multibyte UTF-8 (e.g., non-ASCII user/repo names) this can split a rune and produce a leading replacement character. Pre-existing behavior, but since the surroundingmaxLenlogic is being updated it's worth tightening while you're here.♻️ Proposed rune-safe truncation
- if len(folder) > maxLen { - folder = "..." + folder[len(folder)-maxLen+3:] - } + if runes := []rune(folder); len(runes) > maxLen { + folder = "..." + string(runes[len(runes)-maxLen+3:]) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/ui/repo_selector.go` around lines 864 - 878, The truncation logic in the default branch uses byte slicing (folder[len(folder)-maxLen+3:]) which can cut multibyte UTF‑8 runes; change to rune-safe slicing by converting folder to runes (e.g., r := []rune(folder)), compute the rune length and take the last N runes (N = maxLen-3) before prefixing "..." so the variable folder is rebuilt from runes rather than bytes; update the code that sets folder and the condition that checks len(folder) accordingly (refer to the local variables folder, maxLen and the default case).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/ui/repo_selector.go`:
- Around line 864-878: The truncation logic in the default branch uses byte
slicing (folder[len(folder)-maxLen+3:]) which can cut multibyte UTF‑8 runes;
change to rune-safe slicing by converting folder to runes (e.g., r :=
[]rune(folder)), compute the rune length and take the last N runes (N =
maxLen-3) before prefixing "..." so the variable folder is rebuilt from runes
rather than bytes; update the code that sets folder and the condition that
checks len(folder) accordingly (refer to the local variables folder, maxLen and
the default case).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad33cb40-10f4-4dc2-a4ca-e3db8699152b
📒 Files selected for processing (5)
README.mdinternal/config/loader.gointernal/ui/config_view.gointernal/ui/repo_selector.gointernal/ui/view_layout_test.go
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/ui/view_layout_test.go
- internal/config/loader.go
- internal/ui/config_view.go
…arch Co-authored-by: Ben Schellenberger <bschellenberger2600@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Old default_mode values cause config load failure
- Validate now maps legacy default_mode values with the push- prefix to sync-default before the allowed-value switch, matching prior ParseMode behavior so old configs load.
You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 2be0552. Configure here.
2 participants
Summary
default_modenote.config.tomland TUI Default mode row now use canonicalglobal.default_modestrings (sync-default,sync-all,sync-current-branch,leave-untouched) matchinggit.ParseMode. Removed mistakenpush-*labels that never matched the parser.global.default_modeis validated on load; empty normalizes tosync-default; invalid values error.--rainflag, README); shortened CLAUDE.md maintenance notes.Testing
make test-racemake lintNote
Medium Risk
Touches concurrency/cancellation (scanner + TUI streaming) and config file write semantics via new file locking/atomic replace, which can affect runtime behavior and persistence if edge cases are missed, but changes are localized and test-covered.
Overview
Updates docs (
README.md,CLAUDE.md, CLI long help) to clarify the mode hierarchy, when full branch hydration is triggered (beyond--sync),--pruneprecedence, dry-run behavior, backup-ref naming, and registry vs config locking semantics.Hardens persistence and cancellation:
config.SaveConfig/WriteExampleConfignow use a bounded cross-process lock (.lock) plus PID-scoped atomic replace, andglobal.default_modeis normalized/validated to the canonical values (rejecting legacypush-*). The--rainstreaming TUI path cancels scans earlier, increases channel buffering, and makes repo scanninggitsubprocesses context-cancellable.Refactors the Bubble Tea UI layout to prevent border gaps/cropping and width mis-measurements (new
panel_layout.go, measured list capacity, lipgloss-based path width, ctrl-c handling), and adds focused tests for dry-run prune messaging, concurrent config writes, pre-cancelled scans, and layout invariants.Reviewed by Cursor Bugbot for commit 2be0552. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Documentation
Bug Fixes
New Features
Tests